Skip to content

Implementation for SEP-2350 Client-side scope accumulation in step-up authorization#1591

Open
mikekistler wants to merge 10 commits into
mainfrom
mdk/sep-2350
Open

Implementation for SEP-2350 Client-side scope accumulation in step-up authorization#1591
mikekistler wants to merge 10 commits into
mainfrom
mdk/sep-2350

Conversation

@mikekistler

@mikekistler mikekistler commented May 20, 2026

Copy link
Copy Markdown
Contributor

Motivation and Context

Implements SEP-2350 client-side scope accumulation for step-up authorization in the C# SDK OAuth client flow.

Fixes #1547

What changed

Runtime behavior

  • Updated ClientOAuthProvider to accumulate scopes across re-authorization attempts.
  • On 403 insufficient_scope, the next authorization request now sends the union of:
  • previously requested/granted scopes, and
  • newly challenged scopes from WWW-Authenticate.
  • This prevents losing previously granted permissions during step-up flows.

Thread-safety fix

  • Made the scope accumulator in ClientOAuthProvider thread-safe by protecting mutable scope state with a lock.

Documentation/comments

  • Updated internal auth metadata documentation to reflect SEP-2350 semantics:
  • challenge scopes are authoritative for the current operation,
  • clients should include prior scopes when re-authorizing.

Tests

Updated and added OAuth tests in ModelContextProtocol.AspNetCore.Tests:

  • Updated existing step-up test to validate accumulated scope behavior.
  • Added a new multi-step test to verify scope accumulation across multiple step-up challenges.

All OAuth tests pass across net8.0/net9.0/net10.0 in this branch.

How Has This Been Tested?

Ran auth/scope-step-up against modelcontextprotocol/conformance main (branch install).

  • SEP-2350 check sep-2350-scope-union-on-reauth: SUCCESS
  • Scenario had one unrelated failure (sep-837-application-type-present), but SEP-2350 behavior passes.

Breaking Changes

None.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

mikekistler and others added 3 commits May 19, 2026 16:12
…zation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements SEP-2350 step-up authorization behavior in the C# SDK OAuth client flow by accumulating requested scopes across re-authorization attempts, and updates tests/docs to validate and describe the new semantics.

Changes:

  • Updated ClientOAuthProvider to union previously requested scopes with newly challenged scopes (and added locking for thread-safety).
  • Updated OAuth step-up tests to validate scope accumulation across one and multiple 403 insufficient_scope challenges.
  • Added internal documentation updates (PRM metadata semantics) and a new “run conformance from branch” skill guide.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs Expands step-up OAuth tests to validate accumulated scopes across multiple authorization challenges.
src/ModelContextProtocol.Core/Authentication/ProtectedResourceMetadata.cs Updates internal PRM documentation to describe SEP-2350 scope accumulation expectations.
src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs Implements scope accumulation (union) and adds locking around mutable scope state.
.github/skills/run-conformance-from-branch/SKILL.md Adds workflow documentation for running conformance tests against a GitHub branch build.

Comment thread src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs Outdated
Comment thread .github/skills/run-conformance-from-branch/SKILL.md
Comment thread src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs Outdated
Comment thread src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs
tarekgh and others added 5 commits June 19, 2026 16:43
… no new scope

Per SEP-2350, a step-up authorization should be attempted at least once, but a
repeated insufficient_scope 403 that introduces no scope beyond those already
requested cannot make progress by re-running interactive authorization. Track
whether a step-up has been attempted and treat a subsequent no-new-scope
challenge as a permanent failure instead of prompting the user again for the
same resource and operation.

Also return the current operation scopes as a list of tokens so the new-scope
comparison and accumulation avoid joining and re-splitting the scopes_supported
metadata.

Add tests covering the permanent-failure path and the guarantee that the first
step-up is always allowed even when the challenge reuses an already-requested
scope.
The accumulator holds the union of all scopes requested so far, including the
current operation's scopes after they are merged, so _previouslyRequestedScopes
read slightly off. Rename it to _accumulatedScopes to match what it actually
holds. Internal, no behavioral change.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

@tarekgh tarekgh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed some changes, LGTM otherwise. Please have a look at the changes I pushed just in case you see something. Also, technically the included skill.md file is not part of this PR but no harm if you want to keep or you choose to add it in its own PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SEP-2350: Clarify client-side scope accumulation in step-up authorization

5 participants